Catalog #4714

AlyonaV22 · 2024-10-08T06:44:23Z

mateacademy-ai-mentor

Great job on following the HTML structure and adhering to naming conventions! Just remember to keep the minor points in mind for future improvements. Your effort is commendable, and with a little more attention to detail, your code will be even better! 🚀 Keep up the good work!

✨ Please rate this review in the Platform's notification center. I will be grateful for your feedback! ✨

mateacademy-ai-mentor · 2024-10-08T06:45:17Z

src/index.html

+          <li class="nav__item">
+            <a
+              class="nav__link is-active"
+              href="https://apple.com"


The 'is-active' class should be 'nav__link--active' to follow the BEM methodology as per the checklist. The modifier should be separated from the block or element name by two hyphens.

mateacademy-ai-mentor · 2024-10-08T06:45:17Z

src/index.html

+            <a
+              class="nav__link"
+              href="https://en.wikipedia.org/wiki/Laptop"
+              data-qa="nav-hover"


The 'data-qa' attribute is used for testing purposes. Ensure that its use here is intentional and that it follows the testing strategy of the project.

mateacademy-ai-mentor · 2024-10-08T06:45:17Z

src/index.html

+          <a
+            href="#"
+            class="card__button"
+            data-qa="hover"


The value of the 'data-qa' attribute should be 'card-hover' to maintain consistency across the project, as seen in line 149.

mateacademy-ai-mentor · 2024-10-08T06:45:18Z

src/styles/blocks/card.scss

+  display: grid;
+  place-content: center;
+  place-items: center;
+  row-gap: 48px 46px;


The row-gap property is used to set the vertical spacing between rows, but here it seems to have two values which is not valid for row-gap. If you intended to set both the row and column gaps, use the gap property instead.

mateacademy-ai-mentor · 2024-10-08T06:45:18Z

src/styles/blocks/card.scss

+}
+
+.card {
+  box-sizing: border-box;


According to the Code Style Rules, it's recommended to avoid using box-sizing: border-box; multiple times. Consider creating a common class or using inheritance to apply this property once globally.

mateacademy-ai-mentor · 2024-10-08T06:45:18Z

src/styles/blocks/header.scss

+.is-active::after {
+  position: absolute;
+  content: '';
+  display: block;
+  height: 4px;
+  width: 100%;
+  background-color: $blue-color;
+  border-radius: 8px;
+  bottom: 0;
+}


The ::after pseudo-element is used to create a visual effect under the .is-active class. Ensure that this effect is not causing any unintended layout shifts or accessibility issues. Also, verify that the $blue-color variable is defined in your variables file and is consistent with the design requirements.

mateacademy-ai-mentor · 2024-10-08T06:45:18Z

src/styles/blocks/header.scss

+a[data-qa='nav-hover'] {
+  white-space: nowrap;
+  overflow: hidden;
+}


The white-space and overflow properties are set to prevent the text inside the anchor from wrapping and overflowing. However, these properties are not necessary if the text content fits within the anchor's width. Make sure that it's actually required to use these properties for the [data-qa='nav-hover'] selector.

mateacademy-ai-mentor · 2024-10-08T06:45:18Z

src/styles/blocks/header.scss

+.nav:hover .nav__link {
+  color: $blue-color;
+}


This rule will change the color of .nav__link elements to $blue-color on hover of the .nav block. It might cause an unexpected behavior where all links change color even if only one is hovered. Consider applying the hover effect to .nav__link elements individually unless the group behavior is intentional.

mateacademy-ai-mentor · 2024-10-08T06:45:19Z

src/styles/blocks/stars.scss

+    &:last-child::after {
+      position: absolute;
+      content: 'Reviews: 5';
+      color: $main-text-color;
+      white-space: nowrap;
+      font-size: 10px;
+      line-height: 14px;
+      top: 50%;
+      transform: translateY(-50%);
+      font-weight: 400;
+      right: -70px;
+    }


The use of ::after to add content like 'Reviews: 5' is not recommended for accessibility and semantic reasons. This content seems to be important information that should be part of the HTML document rather than CSS, as it does not appear to be purely decorative. Consider moving this content to the HTML and using an appropriate element and class to style it.

mateacademy-ai-mentor · 2024-10-08T06:45:19Z

src/styles/blocks/stars.scss

+.stars--4 .stars__star:nth-child(-n + 4) {
+  background-image: url(../images/star-active.svg);
+}


The selector '.stars--4 .stars__star:nth-child(-n + 4)' specifies a modifier class 'stars--4', which is good. However, ensure that you have corresponding styles for other possible star ratings (e.g., '.stars--1', '.stars--2', '.stars--3', etc.) to match different rating scenarios. If they are missing, add them to maintain consistency and complete functionality.

BudnikOleksii

Good work 👍
Left some comments, check them. And a lot of tests fail, probably because you haven't fixed max-width for the container

BudnikOleksii · 2024-10-22T08:21:33Z

src/index.html

+          <img
+            class="card__item"
+            src="./images/imac.jpeg"
+            alt="imac"


alt attribute should describe the image content

BudnikOleksii · 2024-10-22T08:21:58Z

src/styles/blocks/card.scss

+  .container {
+    grid-template-columns: repeat(1, 1fr);
+  }
+}
+
+@media (min-width: 488px) {
+  .container {
+    grid-template-columns: repeat(2, 1fr);
+  }
+}
+
+@media (min-width: 768px) {
+  .container {
+    grid-template-columns: repeat(3, 1fr);
+  }
+}
+
+@media (min-width: 1024px) {
+  .container {
+    grid-template-columns: repeat(4, 1fr);


Use nesting in scss

BudnikOleksii · 2024-10-22T08:22:24Z

src/styles/blocks/card.scss

+.card:hover {
+  transform: scale(1.2);
+}
+
+.card:hover .card__title {
+  color: $title-color;
+}
+
+.card__button:hover {


BudnikOleksii · 2024-10-22T08:22:41Z

src/styles/blocks/header.scss

+  height: 40px;
+}
+
+.nav {


Create a separate file per each styles block

BudnikOleksii · 2024-10-22T08:23:06Z

src/styles/blocks/header.scss

+}
+
+.nav {
+  transition: all 300ms ease;


Specify to which properties should transition apply instead of all

BudnikOleksii · 2024-10-22T08:23:20Z

src/styles/blocks/header.scss

+  }
+}
+
+.nav__list {


Use nesting, fix in all places

BudnikOleksii · 2024-10-22T08:24:13Z

src/styles/blocks/header.scss

+a[data-qa='nav-hover'] {
+  white-space: nowrap;
+  overflow: hidden;
+}
+
+a[data-qa='nav-hover']:hover {
+  color: $blue-color;


Don't use data-qa selector, it for tests only, all links should have hover effect

BudnikOleksii · 2024-10-22T08:25:23Z

src/styles/blocks/stars.scss

+  }
+}
+
+.stars--4 .stars__star:nth-child(-n + 4) {


It should be the same functionality as in the stars task, if the modifier is --3 you should show 3 active stars, if --4 - four, and so on

AlyonaV22 added 2 commits October 7, 2024 23:20

create catalog

ca81342

improve style

6e47626

mateacademy-ai-mentor approved these changes Oct 8, 2024

View reviewed changes

BudnikOleksii suggested changes Oct 22, 2024

View reviewed changes

AlyonaV22 added 7 commits October 23, 2024 22:56

improve style

c6d21b0

remove box-sizing:border-box

c8d54e5

add line

e9a9d5a

improve styles

f9d7c29

improve style

19e4b5a

improve code

085d13d

improve code

721801e

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Catalog #4714

Catalog #4714

AlyonaV22 commented Oct 8, 2024

mateacademy-ai-mentor left a comment

mateacademy-ai-mentor Oct 8, 2024

mateacademy-ai-mentor Oct 8, 2024

mateacademy-ai-mentor Oct 8, 2024

mateacademy-ai-mentor Oct 8, 2024

mateacademy-ai-mentor Oct 8, 2024

mateacademy-ai-mentor Oct 8, 2024

mateacademy-ai-mentor Oct 8, 2024

mateacademy-ai-mentor Oct 8, 2024

mateacademy-ai-mentor Oct 8, 2024

mateacademy-ai-mentor Oct 8, 2024

BudnikOleksii left a comment

BudnikOleksii Oct 22, 2024

BudnikOleksii Oct 22, 2024

BudnikOleksii Oct 22, 2024

BudnikOleksii Oct 22, 2024

BudnikOleksii Oct 22, 2024

BudnikOleksii Oct 22, 2024

BudnikOleksii Oct 22, 2024

BudnikOleksii Oct 22, 2024

Catalog #4714

Are you sure you want to change the base?

Catalog #4714

Conversation

AlyonaV22 commented Oct 8, 2024

mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

BudnikOleksii left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment